Thread: Segmentation fault and can`t find where.

Hello, I`ve almost finished the program I am working at ( not commercial, it`s for me ). And I still can`t get what is wrong. I get segmentation fault on 2nd call of specific function. Here is my source code of problematic code, or at least where I think some bad things happen, but really I can`t track them out. With valgrind I was able to find some errors, but please, lend me a hand.
So the idea - I have 2 lists of strings. One for directories, one for files. A function to walk a directory recursively, and to add to front in both lists respectively for files and for dirs. Then do something on these files. Here is my code for the functions I am using and will explain where I am failing.

DList.cpp

Code:

DList* createList(const char* message) {
    DList* dl =  (DList*)malloc(sizeof(DList));
    dl->head = dl->tail = NULL;
    printf("List created at %p; message:[%s]\n", dl, message);
    if ( dl != NULL ) return dl;
    else return NULL;
}

void addFront(const char *data, DList* list) {
    LNode *tmp = NULL;
    if ( list->head == NULL ) {
        tmp = (LNode*) malloc(sizeof(LNode));
        tmp->data = (char*) malloc(strlen(data)+1);
       tmp->begin = tmp->data;
        tmp->data = strndup(data, strlen(data));
        *tmp->data++ = '\0';
        tmp->index = _index++;
        tmp->next = list->head ;
        list->head = list->tail = tmp;
    } else {
        tmp = (LNode*)malloc(sizeof(LNode));
        tmp->data = (char*) malloc(strlen(data)+1);
        tmp->begin = tmp->data;
        tmp->data = strndup(data, strlen(data));
        *tmp->data++ = '\0';
        tmp->index = _index++;
        tmp->next = list->head;
        list->head = tmp;
    }
}

void freeList(DList* list) {
    while ( list->head != NULL ) {
        LNode* removed = list->head;
        list->head = list->head->next;
       free(removed->begin);
        free(removed);
    }
}

void forEach(DList* list, _doSomething doit) {
    LNode* h = list->head;
    while ( h ) {
        doit(h);
        h = h->next;
    }
}

void* printFileFromList(LNode *list) {

    fprintf(stdout, "[%s][%d]\n", list->data, list->index);
}

Dirwalker.cpp

Code:

int init_dirWalk(int argc, char** argv) {
    DList* __dirs = createList("Directories");
    DList* __files = createList("Files");
    int i=1;
    while ( i < argc ) {
        walkDir(__dirs, __files, argv[i++], RECURSIVE);
    }
    forEach(__dirs, printFileFromList);
    forEach(__files, printFileFromList);

    freeList(__files); //empty lists
    freeList(__dirs);
    free(__files); free(__dirs);
    return 0;
}

int isDir(const char *fname) {
    struct stat s;
    stat(fname, &s);
    return S_ISDIR(s.st_mode);
}

void walkDir(DList *dlist, DList* flist, char *basedir, int recursive) {
    DIR* dir;
    struct dirent* ent;
    dir = opendir(basedir);
    if ( dir != NULL ) {
        while ( ent = readdir(dir ) ) {
            if ( (strcmp(ent->d_name, ".") == 0)
                 ||
                 (strcmp(ent->d_name, ".."))==0 ) {
                continue;
            } 
            char entpath[1024] = "";
            sprintf(entpath, "%s%s\0", basedir, ent->d_name);
            if ( isDir(entpath) ) {
                if ( recursive > 0) {
                  sprintf(entpath, "%s/\0", entpath);
                  addFront(entpath, dlist);
                  walkDir(dlist, flist, entpath, RECURSIVE);
                }
            } else {
                    addFront(entpath, flist);
            } 
        } //end while
        closedir(dir);
    } else {
        fprintf(stderr, "\nFailed to walk directory \%s \n", basedir);
        if ( DEBUG ) {
            perror("opendir()");
        }
    } 
}

And finally a simple main.c testing

Code:

int main(int argc, char** argv) {
int c;
    while ( (c=getchar()) != EOF ) {
        getchar();
        switch ( c ) {
        case 'p':
        case 'P':
            init_dirWalk(argc, argv); break;
        default: printf("Press P/p to print\n"); break;
        }
    }
}

What fails? Well after the first press of p/P, it lists and prints the dir/filse. On second press it gives me a segphault. Really no idea what I got messed up. I`ve listed all the functions I am using so I guess, they should be enough for experts to tell where I am failling.

Is there any special reason that you have variable names like __dirs? I note:

Originally Posted by C99 Clause 7.1.3 Paragraph 1 (part)

All identifiers that begin with an underscore and either an uppercase letter or another underscore are always reserved for any use.

That is, those identifiers are reserved to the implementation, so you should not be naming variables with such names unless you are writing a compiler or standard library implementation.

This looks strange:

Code:

tmp->data = (char*) malloc(strlen(data)+1);
tmp->begin = tmp->data;
tmp->data = strndup(data, strlen(data));
*tmp->data++ = '\0';

I would have expected:

Code:

size_t data_len = strlen(data);
tmp->begin = malloc(data_len + 1);
tmp->data = strndup(data, data_len);

It seems confusing to first assign the result of malloc to tmp->data, then get tmp->begin to point to the same thing, then overwrite tmp->data. Furthermore, a quick check shows that strndup null terminates, so there is no need to do so yourself, and in fact your attempt to null terminate looks wrong since it is equivalent to:

Code:

*tmp->data = '\0';
tmp->data++;

which basically makes the whole strndup thing pointless and then makes it harder for you to correctly call free on tmp->data.

Originally Posted by heatblazer

Why sprintf makes this problem - it`s beyound me.

This is one of the symptoms of undefined behaviour.

Additionally, renaming all the .cpp files to be .c, then removing all the casts on malloc calls would be a good idea.